🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

Greptile Summary

This PR brings the Node SDK's retry behavior to parity with the Kotlin, Go, Ruby, and Python SDKs: retries now apply to all requests (not just two path prefixes), 429/503 are retried, Retry-After is honored and capped, POST requests auto-receive an Idempotency-Key for retry safety, backoff is capped at 8 s, and maxRetries is configurable client-wide and per-request.

• HttpClient gains parseRetryAfter, generateIdempotencyKey, a backoff cap, and the configurable MAX_RETRY_ATTEMPTS property; FetchHttpClient removes the isPathRetryable allowlist and routes every verb through fetchRequestWithRetry with per-call maxRetries support.
• ParseError now carries rawHeaders so non-JSON error responses can surface a Retry-After header into the retry loop.
• A comprehensive test suite covers 429, Retry-After (both forms), idempotency key generation/reuse/preservation, ParseError-based retry, maxRetries: 0, and per-request overrides.

Confidence Score: 5/5

Safe to merge — the retry machinery is well-tested, the backoff and jitter logic is correct, and the Retry-After and idempotency-key handling matches the behaviour of other WorkOS SDKs.

The implementation correctly handles the full retry lifecycle: retryable statuses, non-JSON error bodies, Retry-After header parsing (both delay-seconds and HTTP-date), idempotency key injection for POST, and configurable limits at both client and request level. The test suite is thorough and all edge cases are covered. The only gap is that per-request maxRetries cannot be passed to WorkOS.delete and deleteWithBody due to those methods existing signatures, but the client-wide setting still applies to deletes.

src/workos.ts — the delete and deleteWithBody methods do not propagate a per-request maxRetries override; all other files look correct.

Important Files Changed

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant WorkOS
    participant FetchHttpClient
    participant API

    Caller->>WorkOS: get/post/put/patch(path, options)
    WorkOS->>FetchHttpClient: "verb(path, {maxRetries: opts.maxRetries})"
    FetchHttpClient->>FetchHttpClient: fetchRequestWithRetry(url, method, body, headers, maxRetries)
    Note over FetchHttpClient: resolve maxRetryAttempts = maxRetries ?? MAX_RETRY_ATTEMPTS
    loop Attempt 1 to N
        FetchHttpClient->>API: fetch(url)
        alt 2xx Success
            API-->>FetchHttpClient: 200 OK
            FetchHttpClient-->>WorkOS: HttpClientResponse
        else Retryable error
            API-->>FetchHttpClient: 5xx / 429 / timeout
            alt "attempt <= maxRetryAttempts"
                FetchHttpClient->>FetchHttpClient: sleep with Retry-After or backoff+jitter
            else exhausted
                FetchHttpClient-->>WorkOS: throw error
                WorkOS-->>Caller: throw WorkOSException
            end
        end
    end

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant WorkOS
    participant FetchHttpClient
    participant API

    Caller->>WorkOS: get/post/put/patch(path, options)
    WorkOS->>FetchHttpClient: "verb(path, {maxRetries: opts.maxRetries})"
    FetchHttpClient->>FetchHttpClient: fetchRequestWithRetry(url, method, body, headers, maxRetries)
    Note over FetchHttpClient: resolve maxRetryAttempts = maxRetries ?? MAX_RETRY_ATTEMPTS
    loop Attempt 1 to N
        FetchHttpClient->>API: fetch(url)
        alt 2xx Success
            API-->>FetchHttpClient: 200 OK
            FetchHttpClient-->>WorkOS: HttpClientResponse
        else Retryable error
            API-->>FetchHttpClient: 5xx / 429 / timeout
            alt "attempt <= maxRetryAttempts"
                FetchHttpClient->>FetchHttpClient: sleep with Retry-After or backoff+jitter
            else exhausted
                FetchHttpClient-->>WorkOS: throw error
                WorkOS-->>Caller: throw WorkOSException
            end
        end
    end

_{Reviews (5): Last reviewed commit: "fix(retries): scope auto idempotency key..." | Re-trigger Greptile}

@greptileai re: "Retry before parsing" (src/common/net/fetch-client.ts L216-236) — good catch, fixed in 2050d02.

shouldRetryRequest now also retries a ParseError whose rawStatus is in RETRY_STATUS_CODES, so a transient 500/502/503 returning a non-JSON body (e.g. an HTML error page from a proxy) is retried instead of failing on the first attempt. A malformed-JSON body on a non-retryable status (200, 400, 422, …) still surfaces as ParseError as before. Added a test — retries a retryable status with a non-JSON error body — covering a 503 HTML response followed by a success.

The fix looks correct and well-reasoned. shouldRetryRequest now cleanly handles all three error categories:

1. TypeError — network/transport errors (unchanged)
2. HttpClientError with a retryable status — JSON error bodies (unchanged)
3. ParseError with a retryable rawStatus — the new case: non-JSON bodies (e.g. HTML proxy error pages)

One minor observation: getRetryAfterMs only handles HttpClientError and returns null for ParseError, so a 503 HTML response that also carries a Retry-After header won't honor it — it'll fall back to computed backoff instead. That's probably fine in practice (a proxy returning an HTML error page is unlikely to also include a well-formed Retry-After), but worth being aware of if full compliance is ever needed. Not a blocker.

The retries a retryable status with a non-JSON error body test is a solid smoke test for the new branch. LGTM.

Update: I ended up fixing this rather than leaving it. As of 9c71ffa, ParseError carries the response headers (rawHeaders) and getRetryAfterMs reads them for the ParseError path too, so a retryable non-JSON 429/503 that includes a Retry-After header now honors the server-provided delay instead of falling back to backoff. Covered by the honors Retry-After on a retryable non-JSON error body test. Greptile's latest review is 5/5 with no outstanding issues.